Skip to content

fix: wait for background goroutines during shutdown#1756

Merged
AmanGIT07 merged 5 commits into
mainfrom
fix/track-background-goroutines
Jul 15, 2026
Merged

fix: wait for background goroutines during shutdown#1756
AmanGIT07 merged 5 commits into
mainfrom
fix/track-background-goroutines

Conversation

@AmanGIT07

@AmanGIT07 AmanGIT07 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Based on #1747 — merge that first; this base retargets to main after.

Summary

StartServer now waits for the UI server and the audit log listener to
finish shutting down before running deferred cleanup, instead of only
waiting for the connect server.

Changes

  • The log listener, UI server, and connect server run under an
    errgroup; StartServer returns g.Wait(), so deferred cleanup
    (database and service Close() calls) runs only after all three
    finish.
  • All three receive the group context, so a connect server startup
    failure also winds down the UI server and listener.
  • A cancelled context is no longer logged as "log listener failed" on
    normal shutdown.

Technical Details

errgroup.WithContext cancels the group context when any member
returns an error. Only ServeConnect returns a non-nil error, so
g.Wait() returns exactly what StartServer returned before.

Test Plan

🤖 Generated with Claude Code

AmanGIT07 and others added 4 commits July 10, 2026 15:55
@AmanGIT07 @claude
fix: add graceful shutdown to UI server
a5f9b4a 
The UI server ignored the shutdown signal and had no way to stop: it
used the package-level http.ListenAndServe, which offers no Shutdown.
Build a dedicated http.Server on a local mux, watch the context, and
drain requests within the grace period. The local mux also keeps
routes registered by other packages off the UI port.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AmanGIT07 @claude
fix: skip UI shutdown test when admin SPA assets are absent
bcfcf0f 
The dist folder is gitignored except for a placeholder, so in CI the
embedded assets are empty and ServeUI exits before listening. Skip the
shutdown test in that case, matching the condition ServeUI itself
checks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AmanGIT07 @claude
fix: include parse error in ui server host log
98fa948 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AmanGIT07 @claude
fix: track background goroutines and wait for them on shutdown
5f85e5b 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview, Comment Jul 14, 2026 8:14pm

@coderabbitai

coderabbitai Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b615a69a-5e8d-45b9-9dca-799486a2b72b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coveralls

coveralls commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage Report for CI Build 29364880379

Coverage decreased (-0.02%) to 44.843%

Details

  • Coverage decreased (-0.02%) from the base build.
  • Patch coverage: 17 uncovered changes across 1 file (0 of 17 lines covered, 0.0%).
  • 89 coverage regressions across 3 files.

Uncovered Changes

File Changed Covered %
cmd/serve.go 17 0 0.0%

Coverage Regressions

89 previously-covered lines in 3 files lost coverage.

File Lines Losing Coverage Coverage
core/invitation/service.go 53 48.55%
cmd/serve.go 32 0.0%
internal/store/postgres/invitation_repository.go 4 80.3%

Coverage Stats

Coverage Status
Relevant Lines: 37625
Covered Lines: 16872
Line Coverage: 44.84%
Coverage Strength: 12.51 hits per line
💛 - Coveralls

rohilsurana
rohilsurana reviewed Jul 14, 2026
View reviewed changes

@rohilsurana rohilsurana left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the whole chain: the listener returns ctx.Err() on cancel, #1747's ServeUI returns on every path, and ServeConnect's drain is bounded, so g.Wait() cannot hang for more than the grace period. One suggestion inline about signal handling during that window.

Comment thread cmd/serve.go
@AmanGIT07 @claude
fix: restore default signal handling once shutdown starts
9a90116 
A second SIGINT/SIGTERM during the drain was swallowed by the
NotifyContext registration, leaving no way to force-quit a stuck
shutdown. Unregister the handler when shutdown begins so the second
signal gets default handling.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Base automatically changed from fix/ui-server-graceful-shutdown to main July 14, 2026 20:33
@AmanGIT07 AmanGIT07 merged commit d43bcfb into main Jul 15, 2026
8 checks passed
@AmanGIT07 AmanGIT07 deleted the fix/track-background-goroutines branch July 15, 2026 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rohilsurana rohilsurana rohilsurana approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AmanGIT07 @coveralls @rohilsurana